fix(react/testing-library): align fireEvent with Lynx bubbling semantics#2532
Conversation
- TouchEvent-family events (tap, longtap, touchstart, touchmove, touchend, touchcancel, longpress) now default to bubbles: true, matching the Lynx runtime where these events propagate through capture/bubble phases. - Skip read-only Event accessors (bubbles/cancelable/composed) when applying EventInit via Object.assign — those are getters on Event.prototype and reassigning them throws TypeError in strict mode. - Add Event handler property semantics tests covering bind/catch/ capture-bind/capture-catch propagation and bubble defaults across the TouchEvent family.
🦋 Changeset detectedLatest commit: 66f0503 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTouch-event fireEvent helpers now default to bubbles: true; event construction omits Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/testing-library/src/__tests__/events.test.jsx (1)
314-330: Addlongtapto the default-bubbling ancestor test matrix.Line 314 currently validates touchstart/touchmove/touchend/touchcancel/longpress, but
longtapalso changed to defaultbubbles: trueinfire-event.ts. Including it here would close that regression gap.🧪 Proposed test update
- it.each(['touchstart', 'touchmove', 'touchend', 'touchcancel', 'longpress'])( + it.each(['longtap', 'touchstart', 'touchmove', 'touchend', 'touchcancel', 'longpress'])( '%s: bubbles to ancestor handlers by default', (eventName) => { const parent = vi.fn(); const childRef = createRef();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/testing-library/src/__tests__/events.test.jsx` around lines 314 - 330, Update the test matrix in the it.each call so the ancestor-bubbling test covers the 'longtap' event as well: inside the array passed to it.each (currently ['touchstart','touchmove','touchend','touchcancel','longpress']), add 'longtap' so that the loop invokes fireEvent[eventName](childRef.current) for 'longtap' and asserts parent handler was called; this ensures the change in fire-event.ts (default bubbles: true for longtap) is covered by the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react/testing-library/src/__tests__/events.test.jsx`:
- Around line 314-330: Update the test matrix in the it.each call so the
ancestor-bubbling test covers the 'longtap' event as well: inside the array
passed to it.each (currently
['touchstart','touchmove','touchend','touchcancel','longpress']), add 'longtap'
so that the loop invokes fireEvent[eventName](childRef.current) for 'longtap'
and asserts parent handler was called; this ensures the change in fire-event.ts
(default bubbles: true for longtap) is covered by the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e101f9b8-2193-46b8-b249-8fe0d90e315e
📒 Files selected for processing (3)
.changeset/fix-testing-library-tap-bubbles.mdpackages/react/testing-library/src/__tests__/events.test.jsxpackages/react/testing-library/src/fire-event.ts
Merging this PR will not alter performance
Comparing Footnotes
|
Web Explorer#9256 Bundle Size — 900.02KiB (0%).66f0503(current) vs 3920afa main#9255(baseline) Bundle metrics
Bundle size by type
|
| Current #9256 |
Baseline #9255 |
|
|---|---|---|
495.88KiB |
495.88KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#814 Bundle Size — 196.54KiB (0%).66f0503(current) vs e15852f main#811(baseline) Bundle metrics
|
| Current #814 |
Baseline #811 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
44.08% |
44.08% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #814 |
Baseline #811 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.31KiB |
85.31KiB |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7682 Bundle Size — 225.38KiB (0%).66f0503(current) vs e15852f main#7679(baseline) Bundle metrics
|
| Current #7682 |
Baseline #7679 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.57% |
44.57% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7682 |
Baseline #7679 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.63KiB |
79.63KiB |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
React External#799 Bundle Size — 680.27KiB (0%).66f0503(current) vs 3920afa main#798(baseline) Bundle metrics
|
| Current #799 |
Baseline #798 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch fix/testing-library-tap-bubbles Project dashboard
Generated by RelativeCI Documentation Report issue
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/autolink-codegen@0.1.0 ### Minor Changes - Add the Native Autolink codegen package. ([#2601](#2601)) ## create-lynx-extension@0.1.0 ### Minor Changes - Add the Native Autolink create-extension package. ([#2587](#2587)) ### Patch Changes - Use published package versions for scaffolded autolink codegen dependencies instead of workspace placeholders. ([#2628](#2628)) - Fix npm bin symlink entrypoint detection for the create extension CLI. ([#2623](#2623)) ## @lynx-js/react@0.121.0 ### Minor Changes - Support `React.createElement(type, props, children)` API. ([#2360](#2360)) ```jsx React.createElement("view", { style }, <text>hello</text>); // equivalent to <view style={style}> <text>hello</text> </view>; React.createElement(MyComponent, { style }, <view />); // equivalent to <MyComponent style={style}> <view /> </MyComponent>; ``` ### Patch Changes - Clear transient snapshot child props when removed snapshot subtrees are detached, preventing compiled `$*` child references from retaining deleted list holder or list item subtrees after removal. ([#2590](#2590)) - Add `createPortal` for rendering a subtree into a different ReactLynx element identified by a `NodesRef`. ([#2543](#2543)) ```tsx function App() { const [host, setHost] = useState(null); return ( <view> <view ref={setHost} /> {host && createPortal(<text>hi</text>, host)} </view> ); } ``` - Default `fireEvent` to `bubbles: true` for the TouchEvent family in testing-library to match Lynx runtime semantics, and stop reassigning the read-only `Event.prototype` accessors which threw `TypeError` in strict mode. ([#2532](#2532)) - Set `bundle-url` on lazy bundle border elements. ([#2537](#2537)) - Stop warning when `runWorklet` receives an invalid or missing main-thread function object. Invalid worklet contexts are still ignored, but nullish handler values no longer produce noisy `MainThreadFunction: Invalid function object` console output. ([#2586](#2586)) - Retain main-thread worklet context references before offscreen snapshot elements are materialized, so event, ref, gesture, and spread callbacks stay alive until the DOM update path can attach them. ([#2592](#2592)) - Update the @lynx-js/tasm dependency to 0.0.39 and align React template attribute descriptors with it. ([#2643](#2643)) - Avoid retaining transformed nested worklet contexts after worklet transformation. ([#2591](#2591)) Nested worklets transformed by the worklet runtime now keep their context recovery metadata through a weak reference, preventing cached transformed worklet functions from keeping list-item worklet contexts alive. ## @lynx-js/docs-mcp-server@0.2.3 ### Patch Changes - fix(docs-mcp): recursively crawl and register nested llms.txt resources ([#2317](#2317)) ## @lynx-js/rspeedy@0.14.4 ### Patch Changes - feat(qrcode): support get entry from api exposed from rspeedy.env.entries ([#2551](#2551)) - Updated dependencies \[[`ad1f90f`](ad1f90f)]: - @lynx-js/chunk-loading-webpack-plugin@0.3.4 - @lynx-js/web-rsbuild-server-middleware@0.20.4 - @lynx-js/cache-events-webpack-plugin@0.0.3 ## @lynx-js/lynx-bundle-rslib-config@0.3.3 ### Patch Changes - Update the @lynx-js/tasm dependency to 0.0.39 and align React template attribute descriptors with it. ([#2643](#2643)) ## @lynx-js/qrcode-rsbuild-plugin@0.4.7 ### Patch Changes - feat(qrcode): support get entry from api exposed from rspeedy.env.entries ([#2551](#2551)) ## @lynx-js/react-rsbuild-plugin@0.16.2 ### Patch Changes - Updated dependencies \[[`3e627b3`](3e627b3), [`7b8d63c`](7b8d63c), [`13a0776`](13a0776), [`a973c54`](a973c54), [`353b1b7`](353b1b7)]: - @lynx-js/template-webpack-plugin@0.11.1 - @lynx-js/react-refresh-webpack-plugin@0.3.6 - @lynx-js/react-alias-rsbuild-plugin@0.16.2 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-webpack-plugin@0.9.2 - @lynx-js/css-extract-webpack-plugin@0.7.1 ## @lynx-js/web-core@0.20.4 ### Patch Changes - Always clone touch event lists when creating cross-thread events so synthetic touch events only carry structured-clone-safe primitive fields. ([#2636](#2636)) - Conditionally pass Card and Component params based on cardType in background thread. ([#2610](#2610)) - Add bidirectional decode worker heartbreak liveness messages. ([#2599](#2599)) - Add web support for the `<frame>` element by mapping it to `<lynx-view>`. ([#2604](#2604)) - Stop redeclaring `fetch` as a chunk-scope binding. Reusing the host ([#2562](#2562)) `window.fetch` from BTS chunks (instead of capturing the no-op stub the chunk wrapper used to install) lets the renderer issue real network requests. - Updated dependencies \[[`c1db603`](c1db603)]: - @lynx-js/web-elements@0.12.2 - @lynx-js/web-worker-rpc@0.20.4 ## @lynx-js/web-elements@0.12.2 ### Patch Changes - fix: xmarkdown create img incorrectly ([#2540](#2540)) ## @lynx-js/chunk-loading-webpack-plugin@0.3.4 ### Patch Changes - Override `__webpack_require__.e` so a single sync-then chunk load (the ([#2597](#2597)) typical lazy bundle case) bypasses `Promise.all`. It will make first screen in main thread can load lazy bundle synchronously when using dynamic import. ## @lynx-js/react-refresh-webpack-plugin@0.3.6 ### Patch Changes - Widen `@lynx-js/react-webpack-plugin` peer range to include `^0.9.0`. ([#2626](#2626)) ## @lynx-js/template-webpack-plugin@0.11.1 ### Patch Changes - feat(web): enable web binary template by default ([#2545](#2545)) The default encoding format for the web platform template has been changed from JSON to Binary. **Benefits for developers:** - **Smaller output size:** Binary templates are more compact than JSON strings, reducing the final bundle size. - **Faster load performance:** Binary templates parse faster than JSON in the runtime, improving the time-to-interactive for web applications. **How to turn off this feature:** If you encounter any issues with the new binary template format, you can revert to the previous JSON format by setting the environment variable `EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE` to `'false'` or `'0'` before running your build commands. For example: `EXPERIMENTAL_USE_WEB_BINARY_TEMPLATE=false rspeedy build` **Upgrade to `@lynx-js/web-core@0.20.2` could support the new output format** See [`@lynx-js/web-core` Changelog](https://lynx-stack.dev/changelog/lynx-js--web-core) - Run TASM template encoding in a shared `tinypool` worker pool so multi-entry builds encode in parallel and watch-mode rebuilds reuse warm workers. ([#2634](#2634)) - Make `LynxTemplatePlugin.getLynxTemplatePluginHooks` a cross-module singleton so duplicate copies of this package (e.g. from npm hoist conflicts) share the same hooks per compilation. ([#2624](#2624)) - Update the @lynx-js/tasm dependency to 0.0.39 and align React template attribute descriptors with it. ([#2643](#2643)) - Updated dependencies \[[`ee79eff`](ee79eff), [`ded4de9`](ded4de9), [`cf01e94`](cf01e94), [`b989c1c`](b989c1c), [`8417e68`](8417e68)]: - @lynx-js/web-core@0.20.4 ## @lynx-js/react-umd@0.121.0 ## create-rspeedy@0.14.4 ## @lynx-js/react-alias-rsbuild-plugin@0.16.2 ## upgrade-rspeedy@0.14.4 ## @lynx-js/web-rsbuild-server-middleware@0.20.4 ## @lynx-js/web-worker-rpc@0.20.4 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
tap,longtap,touchstart,touchmove,touchend,touchcancel,longpress) now default tobubbles: true, matching the Lynx runtime where every event whose handler signature isEventHandler<BaseTouchEvent<T>>in@lynx-js/typespropagates through capture/bubble phases. Other entries (bgload,transitionend, mouse/key/focus/blur/layout, …) keep their non-bubbling default — Lynx has no symmetric capture-bind/capture-catch API for them.Event.prototypeaccessors (bubbles,cancelable,composed) when applyingEventInitviaObject.assignafter construction — reassigning those getters throwsTypeErrorin strict mode. They're still applied via theEventInitdict.bind/catch/capture-bind/capture-catchhandler semantics and the TouchEvent-family bubble defaults inevents.test.jsx.Test plan
pnpm vitest run src/__tests__/events.test.jsx— 41/41 pass (was previously failing in strict mode on theObject.assign(event, init)call wheninitcarriedbubbles/cancelable/composed)Summary by CodeRabbit
Bug Fixes
Tests
Documentation